-
Notifications
You must be signed in to change notification settings - Fork 680
Human Annotation #6390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Human Annotation #6390
Conversation
Update annotation endpoint to initialize null labels prior to mutation
…yone into feat/human-annotation
implement basic feature flags
add feature flag for human annotation
…conflicts # Conflicts: # app/yarn.lock # fiftyone/zoo/models/manifest-torch.json
resolve merge conflicts from develop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 82
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/packages/core/src/components/ColorModal/SidebarList.tsx (1)
41-46: Fix state type inconsistency: array initialized but updated as object.The
groupOpenstate is initialized as an array but updated as an object, causing type inconsistency. While numeric key access happens to work for both, this is error-prone and violates best practices for consistent state types.Apply this diff to fix the issue by consistently using an object:
- const [groupOpen, setGroupOpen] = React.useState( - new Array(groups.length).fill(true) - ); + const [groupOpen, setGroupOpen] = React.useState( + Object.fromEntries(groups.map((_, idx) => [idx, true])) + );Alternatively, if you prefer to keep it as an array, update line 45 to use array mutation:
const handleGroupClick = (_, idx) => { - setGroupOpen((prev) => ({ ...prev, [idx]: !prev[idx] })); + setGroupOpen((prev) => { + const next = [...prev]; + next[idx] = !next[idx]; + return next; + }); };app/packages/core/src/components/Actions/Selected/hooks.ts (1)
11-27: Add missing dependency to useRecoilCallback.The callback uses
scenefrom the outer scope, but the dependency array is empty. This could lead to stale closures ifscenechanges.Apply this diff:
close(); }, - [] + [close, scene] );
| <div id="modal"></div> | ||
| <div id="colorModal"></div> | ||
| <div id="queryPerformance"></div> | ||
| <div id="annotation" style="display: none;"></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that display: none works correctly for portal target.
React portals can render into hidden containers, but typically the portal target itself should be visible while the portaled content manages its own display state. Verify that setting display: none on the portal target doesn't interfere with the annotation modal's visibility logic.
🏁 Script executed:
#!/bin/bash
# Description: Find components that portal into #annotation to verify display handling
# Search for createPortal or portal usage with "annotation" element
rg -n --type=tsx --type=ts -C3 'createPortal.*annotation|getElementById.*annotation'Length of output: 27
I need to correct the shell script syntax and search for portal usage targeting the annotation element.
🏁 Script executed:
#!/bin/bash
# Description: Find components that portal into #annotation to verify display handling
# Search across TypeScript and JavaScript files for createPortal or getElementById with "annotation"
rg -n -C3 'createPortal.*annotation|getElementById.*annotation' --type-list | grep -i 'typescript\|javascript' || rg -n -C3 'createPortal.*annotation|getElementById.*annotation'Length of output: 1977
🏁 Script executed:
#!/bin/bash
# Alternative: Search for any createPortal usage to understand portal patterns
rg -n 'createPortal' -C2Length of output: 8234
🏁 Script executed:
#!/bin/bash
# Search for usage of the annotation element ID
rg -n 'annotation' --type-list | head -20 || rg -n 'id.*annotation|annotation.*id' -C2Length of output: 389
🏁 Script executed:
#!/bin/bash
# Check file structure in app/packages/app directory
find app/packages/app -type f \( -name '*.ts' -o -name '*.tsx' -o -name '*.js' -o -name '*.jsx' \) | head -20Length of output: 940
🏁 Script executed:
#!/bin/bash
# Search for portal patterns and annotation element more broadly
rg -n 'ReactDOM.createPortal|createPortal' -A5 -B2Length of output: 13312
🏁 Script executed:
#!/bin/bash
# Look for Modal or Portal components that might use annotation
rg -n 'Modal|Portal' -C3 --max-count=15Length of output: 265471
🏁 Script executed:
#!/bin/bash
# Search in HTML files for the annotation element and nearby context
rg -n 'id="annotation"' -C5Length of output: 558
🏁 Script executed:
#!/bin/bash
# Check if there's a CSS file that might override display: none
find app/packages/app -type f \( -name '*.css' -o -name '*.scss' \) | head -10Length of output: 74
🏁 Script executed:
#!/bin/bash
# Search for inline styles or className assignments related to annotation visibility
rg -n 'annotation.*display|display.*annotation|annotation.*visible|annotation.*hidden' -C2Length of output: 346
🏁 Script executed:
#!/bin/bash
# Look for TypeScript/React patterns with portal or modal
rg -n 'useRef.*annotation|usePortal|useModal' -C3Length of output: 9511
🏁 Script executed:
#!/bin/bash
# Final check: search all JavaScript-like files for portal-annotation connection
rg -i 'createportal|portal.*render|render.*portal' -C3 2>/dev/null || echo "Pattern search completed"Length of output: 11021
🏁 Script executed:
#!/bin/bash
# Check the index.html structure more carefully
cat app/packages/app/index.html | head -50Length of output: 771
Action: remove or stop using display:none on the portal target (#annotation) — it will hide portaled annotation modals.
The portal target in app/packages/app/index.html is set to
, and the annotate modals portal into that element (see app/packages/core/src/components/Modal/Sidebar/Annotate/Confirmation/Modal.tsx and SchemaManager/Modal.tsx). Because display:none hides all descendants, any content rendered into #annotation will be invisible unless the target's style is changed at runtime.- Fix options (pick one):
- Remove the inline
display: nonefrom app/packages/app/index.html and let each modal control its own visibility (preferred). - Or, before calling createPortal, explicitly make the target visible (e.g., element.style.display = "block") and hide it again when the modal closes.
- Remove the inline
Files to update:
- app/packages/app/index.html (remove or change style on ).
- Verify annotate modal code in:
- app/packages/core/src/components/Modal/Sidebar/Annotate/Confirmation/Modal.tsx
- app/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/Modal.tsx
🤖 Prompt for AI Agents
In app/packages/app/index.html around line 18 the portal target div has an
inline style display:none which hides any portaled modal content; remove the
inline display:none from <div id="annotation"> (preferred) so each modal
controls its own visibility, or alternately ensure the target is made visible
before createPortal and hidden again when the modal closes; after changing
index.html, verify the annotate modals in
app/packages/core/src/components/Modal/Sidebar/Annotate/Confirmation/Modal.tsx
and
app/packages/core/src/components/Modal/Sidebar/Annotate/SchemaManager/Modal.tsx
still manage visibility correctly and update them if they rely on the target
being hidden.
| if (cleanedValue.startsWith('"') && cleanedValue.endsWith('"')) { | ||
| cleanedValue = cleanedValue.substring(1, headerValue.length - 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect length reference in substring.
Line 50 uses headerValue.length instead of cleanedValue.length when slicing. Since cleanedValue is the variable being checked and modified, the substring operation should use its length.
Apply this diff:
- cleanedValue = cleanedValue.substring(1, headerValue.length - 1);
+ cleanedValue = cleanedValue.substring(1, cleanedValue.length - 1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (cleanedValue.startsWith('"') && cleanedValue.endsWith('"')) { | |
| cleanedValue = cleanedValue.substring(1, headerValue.length - 1); | |
| if (cleanedValue.startsWith('"') && cleanedValue.endsWith('"')) { | |
| cleanedValue = cleanedValue.substring(1, cleanedValue.length - 1); |
🤖 Prompt for AI Agents
In app/packages/core/src/client/util.ts around lines 49 to 50, the substring
call uses headerValue.length instead of cleanedValue.length; replace the end
index with cleanedValue.length - 1 so the slice matches the checked variable
(i.e., use cleanedValue.substring(1, cleanedValue.length - 1)) to correctly
remove the surrounding quotes.
| {containerRef.current && ( | ||
| <LighterSetupImpl containerRef={containerRef} sceneId={sceneId} /> | ||
| )} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent null sceneId mismatch; guard before initializing PIXI
sceneId is typed as string | null but is passed to LighterSetupImpl which requires string. This can compile-time fail or pass undefined at runtime. Guard rendering until sceneId is available, or early-return in LighterSetupImpl.
Apply one of these diffs (Option A preferred).
Option A — guard at call site:
- {containerRef.current && (
- <LighterSetupImpl containerRef={containerRef} sceneId={sceneId} />
- )}
+ {containerRef.current && sceneId && (
+ <LighterSetupImpl containerRef={containerRef} sceneId={sceneId} />
+ )}Option B — early return in LighterSetupImpl:
const LighterSetupImpl = (props: {
containerRef: React.RefObject<HTMLDivElement>;
- sceneId: string;
+ sceneId: string;
}) => {
const { containerRef, sceneId } = props;
+ if (!containerRef.current || !sceneId) return null;Also applies to: 101-104
🤖 Prompt for AI Agents
In app/packages/core/src/components/Modal/Lighter/LighterSampleRenderer.tsx
around lines 94-96 (and similarly lines 101-104), sceneId is typed string | null
but is passed into LighterSetupImpl which requires a string; guard rendering
until sceneId is non-null (preferred Option A): change the JSX to only render
<LighterSetupImpl ... /> when containerRef.current && sceneId are truthy, or
alternatively (Option B) add an early return/throw in LighterSetupImpl when
sceneId is null and update its prop types to reflect non-null after the check;
ensure TypeScript types align so a string is always passed at call sites.
| .getValue(); | ||
|
|
||
| if (isTooltipLocked || !scene) { | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix event listener cleanup bug.
Line 94 calls scene?.on(LIGHTER_EVENTS.OVERLAY_BOUNDS_CHANGED, handler) instead of scene?.off(...). This registers the handler again during cleanup instead of removing it, causing memory leaks and duplicate event handling.
Apply this diff:
return () => {
- scene?.on(LIGHTER_EVENTS.OVERLAY_BOUNDS_CHANGED, handler);
+ scene?.off(LIGHTER_EVENTS.OVERLAY_BOUNDS_CHANGED, handler);
scene?.off(LIGHTER_EVENTS.OVERLAY_DRAG_MOVE, handler);
scene?.off(LIGHTER_EVENTS.OVERLAY_RESIZE_MOVE, handler);
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/packages/core/src/components/Modal/Lighter/useLighterTooltipEventHandler.ts
around lines 93 to 96, the cleanup currently calls
scene?.on(LIGHTER_EVENTS.OVERLAY_BOUNDS_CHANGED, handler) which re-registers the
handler instead of removing it; change that call to
scene?.off(LIGHTER_EVENTS.OVERLAY_BOUNDS_CHANGED, handler) so the event listener
is properly removed during cleanup to prevent memory leaks and duplicate
handling.
| const versionToken = useMemo(() => { | ||
| const isoTimestamp = parseTimestamp( | ||
| currentSample?.last_modified_at | ||
| )?.toISOString(); | ||
|
|
||
| // server doesn't like the iso timestamp ending in 'Z' | ||
| if (isoTimestamp?.endsWith("Z")) { | ||
| return isoTimestamp.substring(0, isoTimestamp.length - 1); | ||
| } else { | ||
| return isoTimestamp; | ||
| } | ||
| }, [currentSample?.last_modified_at]); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guarantee a valid versionToken; avoid sending If-Match: "undefined".
If last_modified_at is absent, versionToken becomes undefined, and the request will send If-Match: "undefined", likely causing a 412/400. Guard and fail fast with a user-visible error.
Apply:
const handlePatchSample = useCallback(
async (sampleDeltas: JSONDeltas): Promise<boolean> => {
if (sampleDeltas.length > 0) {
- try {
+ try {
+ if (!datasetId || !currentSample?._id || !versionToken) {
+ console.error("missing datasetId, sampleId, or version token");
+ setSnackbarErrors(["Unable to save changes. Please retry."]);
+ return false;
+ }
const response = await patchSample({
datasetId,
sampleId: currentSample._id,
deltas: sampleDeltas,
versionToken,
});Also applies to: 56-61
| async initialize(canvas: HTMLCanvasElement): Promise<PIXI.Application> { | ||
| if (this.isInitialized && this.app) { | ||
| return this.app; | ||
| } | ||
|
|
||
| if (this.initPromise) { | ||
| await this.initPromise; | ||
| return this.app!; | ||
| } | ||
|
|
||
| this.initPromise = this.performInitialization(canvas); | ||
| await this.initPromise; | ||
| return this.app!; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for initialization failures.
The non-null assertions on lines 43 and 48 (this.app!) assume initialization always succeeds. If performInitialization throws an error, this.app remains null, and these assertions would return null when the caller expects a valid PIXI.Application, potentially causing issues downstream.
Consider adding proper error handling:
async initialize(canvas: HTMLCanvasElement): Promise<PIXI.Application> {
if (this.isInitialized && this.app) {
return this.app;
}
if (this.initPromise) {
await this.initPromise;
- return this.app!;
+ if (!this.app) {
+ throw new Error("PIXI application initialization failed");
+ }
+ return this.app;
}
this.initPromise = this.performInitialization(canvas);
- await this.initPromise;
- return this.app!;
+ try {
+ await this.initPromise;
+ if (!this.app) {
+ throw new Error("PIXI application initialization failed");
+ }
+ return this.app;
+ } catch (error) {
+ // Reset promise so next call can retry
+ this.initPromise = null;
+ throw error;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async initialize(canvas: HTMLCanvasElement): Promise<PIXI.Application> { | |
| if (this.isInitialized && this.app) { | |
| return this.app; | |
| } | |
| if (this.initPromise) { | |
| await this.initPromise; | |
| return this.app!; | |
| } | |
| this.initPromise = this.performInitialization(canvas); | |
| await this.initPromise; | |
| return this.app!; | |
| } | |
| async initialize(canvas: HTMLCanvasElement): Promise<PIXI.Application> { | |
| if (this.isInitialized && this.app) { | |
| return this.app; | |
| } | |
| if (this.initPromise) { | |
| await this.initPromise; | |
| if (!this.app) { | |
| throw new Error("PIXI application initialization failed"); | |
| } | |
| return this.app; | |
| } | |
| this.initPromise = this.performInitialization(canvas); | |
| try { | |
| await this.initPromise; | |
| if (!this.app) { | |
| throw new Error("PIXI application initialization failed"); | |
| } | |
| return this.app; | |
| } catch (error) { | |
| // Reset promise so next call can retry | |
| this.initPromise = null; | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/packages/lighter/src/renderer/SharedPixiApplication.ts around lines 36 to
49, the initialize method assumes performInitialization always succeeds and uses
non-null assertions (this.app!) which can return null if performInitialization
throws; update initialize to catch errors from performInitialization,
clear/reset this.initPromise on failure, and rethrow the error so callers see
the failure; also, after awaiting an existing initPromise verify this.app is set
and throw a descriptive error if it's still null instead of returning this.app!,
ensuring callers never receive a null application.
| <ActionButton | ||
| onClick={() => | ||
| !action.isDisabled && | ||
| handleActionClick(action.onClick) | ||
| } | ||
| isdisabled={String(action.isDisabled)} | ||
| isactive={String(action.isActive)} | ||
| size="small" | ||
| > | ||
| {action.icon} | ||
| </ActionButton> | ||
| </Tooltip> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use proper disabled prop for accessibility and UX.
The button checks isDisabled in the onClick handler (line 237) but doesn't pass a disabled prop to the IconButton. This means disabled buttons still show hover effects and appear interactive, which is confusing to users and creates accessibility issues.
Apply this diff to properly disable the button:
<ActionButton
onClick={() =>
- !action.isDisabled &&
- handleActionClick(action.onClick)
+ action.onClick()
}
isdisabled={String(action.isDisabled)}
isactive={String(action.isActive)}
size="small"
+ disabled={action.isDisabled}
>Note: With the disabled prop, you no longer need the isDisabled check in onClick.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <ActionButton | |
| onClick={() => | |
| !action.isDisabled && | |
| handleActionClick(action.onClick) | |
| } | |
| isdisabled={String(action.isDisabled)} | |
| isactive={String(action.isActive)} | |
| size="small" | |
| > | |
| {action.icon} | |
| </ActionButton> | |
| </Tooltip> | |
| <ActionButton | |
| onClick={() => | |
| handleActionClick(action.onClick) | |
| } | |
| isdisabled={String(action.isDisabled)} | |
| isactive={String(action.isActive)} | |
| size="small" | |
| disabled={action.isDisabled} | |
| > | |
| {action.icon} | |
| </ActionButton> |
| setPolylinePointTransforms((prev) => { | ||
| const newTransforms = [...(prev[labelId] || []), ...newPolyline]; | ||
|
|
||
| // Remove duplicates | ||
| const uniqueTransforms = newTransforms.filter( | ||
| (transform, index, self) => | ||
| index === | ||
| self.findIndex( | ||
| (t) => | ||
| t.segmentIndex === transform.segmentIndex && | ||
| t.pointIndex === transform.pointIndex | ||
| ) | ||
| ); | ||
|
|
||
| return { | ||
| ...prev, | ||
| [labelId]: uniqueTransforms, | ||
| }; | ||
| }); | ||
|
|
||
| // Get out of segmenting mode | ||
| setSegmentState({ | ||
| isActive: false, | ||
| vertices: [], | ||
| currentMousePosition: null, | ||
| isClosed: false, | ||
| }); | ||
|
|
||
| setIsActivelySegmenting(false); | ||
|
|
||
| setTempPolylines([]); | ||
| }, [tempPolylines, polylineEffectivePoints, tempLabelId]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Editing bug: effect re-runs on vertex drag, keeps stale transforms, and clears tempPolylines.
- Dedup keeps the first occurrence, discarding updated positions.
setTempPolylines([])removes the just-created polyline and its markers, breaking follow-up edits.
- setPolylinePointTransforms((prev) => {
- const newTransforms = [...(prev[labelId] || []), ...newPolyline];
-
- // Remove duplicates
- const uniqueTransforms = newTransforms.filter(
- (transform, index, self) =>
- index ===
- self.findIndex(
- (t) =>
- t.segmentIndex === transform.segmentIndex &&
- t.pointIndex === transform.pointIndex
- )
- );
-
- return {
- ...prev,
- [labelId]: uniqueTransforms,
- };
- });
+ setPolylinePointTransforms((prev) => {
+ const merged = [...(prev[labelId] || []), ...newPolyline];
+ // Keep the last occurrence (latest position) for each (segmentIndex, pointIndex)
+ const byKey = new Map<string, PolylinePointTransform>();
+ for (const t of merged) byKey.set(`${t.segmentIndex}:${t.pointIndex}`, t);
+ return { ...prev, [labelId]: Array.from(byKey.values()) };
+ });
@@
- // Get out of segmenting mode
+ // Get out of segmenting mode (keep tempPolylines so user can edit vertices)
setSegmentState({
isActive: false,
vertices: [],
currentMousePosition: null,
isClosed: false,
});
setIsActivelySegmenting(false);
- setTempPolylines([]);
- }, [tempPolylines, polylineEffectivePoints, tempLabelId]);
+ // Do not clear tempPolylines here; it is the source for editable markers.
+ }, [tempPolylines, polylineEffectivePoints, tempLabelId, selectedLabelForAnnotation?._id]);If you intend to clear after committing elsewhere, do it in that commit path, not in this synchronization effect.
🤖 Prompt for AI Agents
In app/packages/looker-3d/src/annotation/SegmentPolylineRenderer.tsx around
lines 520 to 551, the synchronization effect de-duplicates transforms by keeping
the first occurrence (which discards updated vertex positions), clears
tempPolylines inside the effect (which removes the newly created polyline and
its markers), and re-runs on vertex drags due to its dependencies; fix by
changing the dedup logic to keep the latest transform per (segmentIndex,
pointIndex) (e.g., build a Map keyed by `${segmentIndex}:${pointIndex}` and
overwrite so the last occurrence wins, or iterate newTransforms in reverse and
keep first-seen), remove the setTempPolylines([]) call from this effect and
instead clear tempPolylines in the explicit commit/finish handler where the
polyline is finalized, and if tempPolylines is only used to derive newPolyline
ensure the effect dependencies do not include transient drag state (remove
tempPolylines from the deps or derive newPolyline inside the commit path) so the
effect does not re-run on every vertex drag.
| import { useAtomValue } from "jotai"; | ||
| import { useCallback, useEffect, useMemo, useRef, useState } from "react"; | ||
| import { useRecoilValue, useSetRecoilState } from "recoil"; | ||
| import { Fo3dErrorBoundary } from "./ErrorBoundary"; | ||
| import { MediaTypePcdComponent } from "./MediaTypePcd"; | ||
| import { ActionBar } from "./action-bar"; | ||
| import { Container } from "./containers"; | ||
| import { Fo3dErrorBoundary } from "./ErrorBoundary"; | ||
| import { Leva } from "./fo3d/Leva"; | ||
| import { MediaTypeFo3dComponent } from "./fo3d/MediaTypeFo3d"; | ||
| import { useHotkey } from "./hooks"; | ||
| import { MediaTypePcdComponent } from "./MediaTypePcd"; | ||
| import { | ||
| clearTransformStateSelector, | ||
| currentActionAtom, | ||
| fo3dContainsBackground, | ||
| isColormapModalOpenAtom, | ||
| isGridOnAtom, | ||
| isLevaConfigPanelOnAtom, | ||
| selectedLabelForAnnotationAtom, | ||
| } from "./state"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify necessity of new imports.
Several new imports were added (clearTransformStateSelector, selectedLabelForAnnotationAtom) that don't appear to be used in the visible code changes. Ensure these are actually needed or will be used in related functionality not shown in this diff.
🏁 Script executed:
#!/bin/bash
# Search for usage of the newly imported state atoms in Looker3d.tsx
rg -n "clearTransformStateSelector|selectedLabelForAnnotationAtom" app/packages/looker-3d/src/Looker3d.tsxLength of output: 70
Remove unused imports from Looker3d.tsx.
The verification confirms that clearTransformStateSelector and selectedLabelForAnnotationAtom are imported but not used anywhere in the file. Remove both from the import statement (lines 18 and 24) unless they will be used immediately in this same PR.
🤖 Prompt for AI Agents
In app/packages/looker-3d/src/Looker3d.tsx around lines 7 to 25, the imports
clearTransformStateSelector and selectedLabelForAnnotationAtom are unused;
remove them from the import list (delete the identifiers from the brace-import)
so the file no longer imports unused symbols and update any trailing
commas/spacing if needed.
| ); | ||
| const isDynamicGroup = useRecoilValue(fos.isDynamicGroup); | ||
| const parentMediaType = useRecoilValue(fos.parentMediaTypeSelector); | ||
| const mode = useAtomValue(fos.modalMode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused variable or use it.
The mode variable is read from state but never used in the component. This is dead code that should either be removed or put to use.
If unused:
- const mode = useAtomValue(fos.modalMode);If it's intended for future use or debugging, consider adding an ESLint disable comment:
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const mode = useAtomValue(fos.modalMode);🤖 Prompt for AI Agents
In app/packages/looker-3d/src/Looker3d.tsx around line 42, the const mode =
useAtomValue(fos.modalMode); is declared but never used; either remove this line
to eliminate dead code, or use the value where intended (e.g., in conditional
rendering or logic) — if it must remain unused temporarily, add an inline ESLint
disable comment above it to suppress the no-unused-vars rule.
What changes are proposed in this pull request?
Human Annotation Feature Branch
What areas of FiftyOne does this PR affect?
fiftyonePython library changesSummary by CodeRabbit
New Features
Improvements
Documentation